Skip to content

Conversation

@bogui56
Copy link
Contributor

@bogui56 bogui56 commented Dec 16, 2024

Set Clusterizer to split common mode noise patterns into zsize = 1 clusters and add machinery to reject them in PHCAseeding

PR Summary: Reject zsize==1 clusters in TPC seeding and cap seed propagation

Motivation / Context

TPC clustering can produce common-mode noise patterns that split into clusters with zsize==1 (single z-bin width), reducing seed quality. This PR filters such clusters during track seeding reconstruction and introduces a cap on the number of seeds propagated through the Kalman filter.

Key Changes

  • PHCASeeding rejection: Add reject_zsize1_clusters(true) flag in Tracking_Reco_TpcSeed_run2pp() to filter out single-z-bin clusters during track seeding
  • Seed count limit: Introduce set_max_seeds(5000) on PHSimpleKFProp to cap the maximum number of seeds propagated during Kalman filter expansion
  • Modifications limited to common/Trkr_Reco.C (+4/-2 lines)

Potential Risk Areas

  • Reconstruction behavior changes: Rejecting zsize==1 clusters alters track seed demographics and may affect downstream tracking efficiency
  • Low pT performance: Discussion comments note a small efficiency loss at low transverse momentum from the zsize==1 rejection; impact is currently considered acceptable
  • Inconsistent seeding configuration: The reject_zsize1_clusters flag is only explicitly added to Tracking_Reco_TpcSeed_run2pp(); multiple other seeding functions in the file (for cosmics, zero-field, iterative seeding) may not have this applied, potentially creating inconsistency across reconstruction paths
  • Seed cap implications: The 5000 seed limit may suppress rare high-multiplicity events if seed propagation would naturally exceed this threshold

Possible Future Improvements

  • Apply the reject_zsize1_clusters flag consistently across all TPC seeding functions in the macro suite
  • Centralize seeding initialization as suggested in PR discussion to ensure uniform module/flag configuration across reconstruction macros
  • Monitor low-pT tracking efficiency in QA to quantify performance trade-off
  • Document typical seed counts to assess whether 5000 is an appropriate cap

Note: AI may make mistakes in reconstructing algorithm details or impact assessment. Cross-reference with actual tracking performance QA and discuss with physics/detector experts before deployment.

@sphenix-jenkins-ci
Copy link

For repository maintainers, please start the CI check manually (feedback)

This is an automatic message to assist manually starting CI check for this pull request, commit 921e604743be291ba2820bf23d3cbe1cab4efd2b. macros pull request require a manual start for CI checks, in particular selecting which coresoftware and calibrations versions to check against this macros pull request.

sPHENIX software maintainers: please make your input here and start the Build:

build

Note:

  1. if needed, fill in the pull request ID for the coresoftware pull request, e.g. origin/pr/1697/merge for PR#1697 in sha_coresoftware. Default is to check with the master branch.
  2. click Build button at the end of the long web page to start the test

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@osbornjd osbornjd marked this pull request as draft January 21, 2025 17:25
@sphenix-jenkins-ci
Copy link

For repository maintainers, please start the CI check manually (feedback)

This is an automatic message to assist manually starting CI check for this pull request, commit 6b09d9cd04f65b5dab93a658be9e9200042978b6. macros pull request require a manual start for CI checks, in particular selecting which coresoftware and calibrations versions to check against this macros pull request.

sPHENIX software maintainers: please make your input here and start the Build:

build

Note:

  1. if needed, fill in the pull request ID for the coresoftware pull request, e.g. origin/pr/1697/merge for PR#1697 in sha_coresoftware. Default is to check with the master branch.
  2. click Build button at the end of the long web page to start the test

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@bogui56 bogui56 marked this pull request as ready for review February 13, 2025 15:24
@osbornjd
Copy link
Contributor

Should we add this flag to our reco macros too? Better yet, is it time for us to have a centralized function call for our seeding reconstruction?

@sPHENIX-GitHub-l
Copy link

sPHENIX-GitHub-l commented Feb 13, 2025 via email

@osbornjd
Copy link
Contributor

For example we are not currently calling the function Tracking_Reco_TrackSeed() in our reconstruction macros, e.g. here. The seeding may be at a point where it has converged enough to have it a central place, which would also make it such that we would have a consistent set of modules/flags that the macros by default call

@bogui56
Copy link
Contributor Author

bogui56 commented Feb 13, 2025

Ah, that way, well for me that would be ok. Modulo the changed Michael and David will still come up with...

@osbornjd
Copy link
Contributor

It does cause a small loss in efficiency at low pT
https://nbviewer.org/github/sPHENIX-Collaboration/QA-gallery/blob/jenkins-sPHENIX-test-tracking-low-occupancy-qa-6871-test-tracking_Event400_Sum28/QA-tracking.ipynb
but that is probably not an issue right now

@sphenix-jenkins-ci
Copy link

For repository maintainers, please start the CI check manually (feedback)

This is an automatic message to assist manually starting CI check for this pull request, commit aba70f9b802fd3c5a8e69889b684b1a671840af0. macros pull request require a manual start for CI checks, in particular selecting which coresoftware and calibrations versions to check against this macros pull request.

sPHENIX software maintainers: please make your input here and start the Build:

build

Note:

  1. if needed, fill in the pull request ID for the coresoftware pull request, e.g. origin/pr/1697/merge for PR#1697 in sha_coresoftware. Default is to check with the master branch.
  2. click Build button at the end of the long web page to start the test

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 19, 2026

📝 Walkthrough

Walkthrough

Modifies common/Trkr_Reco.C to add cluster rejection filtering to PHCASeeding seeders and enforce a maximum seed limit on PHSimpleKFProp propagation, adding two new methods for these configurations.

Changes

Cohort / File(s) Summary
Seeding Configuration
common/Trkr_Reco.C
Adds reject_zsize1_clusters(bool) method to PHCASeeding with four instances enabling z-size-1 cluster rejection; adds set_max_seeds(int) method to PHSimpleKFProp with max seed limit of 5000 applied in two propagation call sites

Comment @coderabbitai help to get the list of available commands and usage tips.

@sphenix-jenkins-ci
Copy link

For repository maintainers, please start the CI check manually (feedback)

This is an automatic message to assist manually starting CI check for this pull request, commit dac2f9f58c568ee68d5422baa19c3c5c2fee714b. macros pull request require a manual start for CI checks, in particular selecting which coresoftware and calibrations versions to check against this macros pull request.

sPHENIX software maintainers: please make your input here and start the Build:

build

Note:

  1. if needed, fill in the pull request ID for the coresoftware pull request, e.g. origin/pr/1697/merge for PR#1697 in sha_coresoftware. Default is to check with the master branch.
  2. click Build button at the end of the long web page to start the test

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@osbornjd osbornjd merged commit dc8a5a6 into sPHENIX-Collaboration:master Jan 20, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants